Top n queries historical queries from local index and time range #84
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Emily Guo <[email protected]> Handle indexing errors and accurate search parsing Signed-off-by: Emily Guo <[email protected]> Correctly filter in window queries when from and to exist Signed-off-by: Emily Guo <[email protected]> Remove search requests from top n queries Signed-off-by: Emily Guo <[email protected]> Removed comments Signed-off-by: Emily Guo <[email protected]> Comments for new functions Signed-off-by: Emily Guo <[email protected]> Comments for new functions Signed-off-by: Emily Guo <[email protected]> Updated comments Signed-off-by: Emily Guo <[email protected]>
Signed-off-by: Emily Guo <[email protected]>
LilyCaroline17
requested review from
ansjcy,
jainankitk,
deshsidd and
dzane17
as code owners
Signed-off-by: Emily Guo <[email protected]>
Signed-off-by: Emily Guo <[email protected]>
|
Looks like one of the build is failing please take a look |
Signed-off-by: Emily Guo <[email protected]>
ansjcy
reviewed
Sep 5, 2024
| end = DateTime.now(DateTimeZone.UTC); | ||
| } | ||
| DateTime curr = start; | ||
| while (curr.compareTo(end.plusDays(1).withTimeAtStartOfDay()) < 0) { |
There was a problem hiding this comment.
Note: We are making assumptions on the index pattern that it will rollover every day. We should add this assumption with a check in the configuration API as well. And we should not allow user to change this rollover period and only allow them to change the prefix pattern. In that case
MatchQueryBuilder excludeQuery = QueryBuilders.matchQuery("indices", "top_queries*");
would become something like
MatchQueryBuilder excludeQuery = QueryBuilders.matchQuery("indices", TOP_N_QUERIES_INDEX_PATTERN + "*");
ansjcy
reviewed
Sep 5, 2024
src/main/java/org/opensearch/plugin/insights/core/service/QueryInsightsService.java
Outdated
Show resolved
Hide resolved
ansjcy
reviewed
Sep 5, 2024
src/main/java/org/opensearch/plugin/insights/core/service/QueryInsightsService.java
Outdated
Show resolved
Hide resolved
ansjcy
reviewed
Sep 5, 2024
src/main/java/org/opensearch/plugin/insights/core/service/TopQueriesService.java
Outdated
Show resolved
Hide resolved
ansjcy
reviewed
Sep 5, 2024
| return Arrays.stream(indices).noneMatch(index -> { | ||
| if (index instanceof String) { | ||
| String indexString = (String) index; | ||
| return indexString.contains("top_queries"); |
There was a problem hiding this comment.
Also we are basically saying ignoring any requests made on the top_queries index. it might be worth discussing it further whether we want to do this.
ansjcy
reviewed
Sep 5, 2024
src/main/java/org/opensearch/plugin/insights/core/service/TopQueriesService.java
Outdated
Show resolved
Hide resolved
ansjcy
reviewed
Sep 5, 2024
...va/org/opensearch/plugin/insights/rules/transport/top_queries/TransportTopQueriesAction.java
Outdated
Show resolved
Hide resolved
…ilters to historical, and rename functions Signed-off-by: Emily Guo <[email protected]>
Signed-off-by: Emily Guo <[email protected]>
Signed-off-by: Emily Guo <[email protected]>
ansjcy
approved these changes
Sep 5, 2024
|
Thanks for making the changes!
|
opensearch-trigger-bot bot
pushed a commit
that referenced
this pull request
Sep 5, 2024
* Added support for historical top n queries Signed-off-by: Emily Guo <[email protected]> Handle indexing errors and accurate search parsing Signed-off-by: Emily Guo <[email protected]> Correctly filter in window queries when from and to exist Signed-off-by: Emily Guo <[email protected]> Remove search requests from top n queries Signed-off-by: Emily Guo <[email protected]> Removed comments Signed-off-by: Emily Guo <[email protected]> Comments for new functions Signed-off-by: Emily Guo <[email protected]> Comments for new functions Signed-off-by: Emily Guo <[email protected]> Updated comments Signed-off-by: Emily Guo <[email protected]> * Added unit tests Signed-off-by: Emily Guo <[email protected]> * Update LocalIndexExporter.java Signed-off-by: Emily Guo <[email protected]> * Update LocalIndexReaderTests.java Signed-off-by: Emily Guo <[email protected]> * Update QueryInsightsReaderFactoryTests.java Signed-off-by: Emily Guo <[email protected]> * Address comments, change getTimeRange into getFrom and getTo, apply filters to historical, and rename functions Signed-off-by: Emily Guo <[email protected]> * Fix test cases Signed-off-by: Emily Guo <[email protected]> --------- Signed-off-by: Emily Guo <[email protected]> Signed-off-by: Emily Guo <[email protected]> Signed-off-by: Emily Guo <[email protected]> (cherry picked from commit 4d896cc) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
ansjcy
pushed a commit
that referenced
this pull request
Sep 5, 2024
#105) * Added support for historical top n queries Handle indexing errors and accurate search parsing Correctly filter in window queries when from and to exist Remove search requests from top n queries Removed comments Comments for new functions Comments for new functions Updated comments * Added unit tests * Update LocalIndexExporter.java * Update LocalIndexReaderTests.java * Update QueryInsightsReaderFactoryTests.java * Address comments, change getTimeRange into getFrom and getTo, apply filters to historical, and rename functions * Fix test cases --------- (cherry picked from commit 4d896cc) Signed-off-by: Emily Guo <[email protected]> Signed-off-by: Emily Guo <[email protected]> Signed-off-by: Emily Guo <[email protected]> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
When the local index exporter is enabled, historical top N queries data is stored there. Previously, obtaining these historical queries required manually searching through each index. To simplify this process for users, we've introduced two parameters: 'from' and 'to', to the top n queries request. This allows users to define the time range for retrieving historical data. Please ensure that from and to are in ISO format.
For example, you could make this following call to retrieve the top N queries from August 25, 2024, at 15:00 UTC to August 30, 2024, at 17:00 UTC.
curl -X GET "http://localhost:9200/_insights/top_queries?from=2024-08-25T15:00:00.000Z&to=2024-08-30T17:00:00.000Z"Issues Resolved
Closes #12
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.